Skip to content

bootstrap: Prefer cfg!(not(test)) when skipping code paths during unit tests#159705

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
Zalathar:cfg-test
Jul 22, 2026
Merged

bootstrap: Prefer cfg!(not(test)) when skipping code paths during unit tests#159705
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
Zalathar:cfg-test

Conversation

@Zalathar

Copy link
Copy Markdown
Member

Bootstrap contains a few code paths that need to be stubbed out when running bootstrap's unit tests, so that they don't download files or cause other problems. (That's unfortunate, but fixing it is outside the scope of this PR.)

Currently those code paths are skipped using #[cfg(test)] and #[cfg(not(test))], which does achieve the desired effect, but has a few downsides. The stubbed-out code triggers dead-code and unused-import warnings, and makes IDE navigation harder when rust-analyzer treats cfg(test) as true when analyzing the code.

If we switch to using cfg!(test) and cfg!(not(test)) to “dynamically” skip the relevant code paths, it still gets skipped, but the skipped code is still considered live for the purpose of dead-code warnings and IDE analysis.

@rustbot

rustbot commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR changes how GCC is built. Consider updating src/bootstrap/download-ci-gcc-stamp.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 22, 2026
@rustbot

rustbot commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: bootstrap
  • bootstrap expanded to 6 candidates
  • Random selection from Mark-Simulacrum, clubby789

@Kobzol

Kobzol commented Jul 22, 2026

Copy link
Copy Markdown
Member

This is much better than repeating function signatures or expecting dead code on top of imports. Thank you!

r? @Kobzol

@bors r+ rollup

@rust-bors

rust-bors Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

📌 Commit db20e5d has been approved by Kobzol

It is now in the queue for this repository.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 22, 2026
@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 22, 2026
@rust-log-analyzer

This comment has been minimized.

@Zalathar

Copy link
Copy Markdown
Member Author

The clippy false-positive for assert!(cfg!(..)) was supposedly fixed in rust-lang/rust-clippy#7319, but I guess it broke again at some point?

@Kobzol

Kobzol commented Jul 22, 2026

Copy link
Copy Markdown
Member

@bors r-

@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 22, 2026
@rust-bors

rust-bors Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

View changes since this unapproval

Assertions have been added to some code paths that were previously not built
during `cfg(test)`, to make sure they aren't accidentally executed.
@Zalathar

Copy link
Copy Markdown
Member Author

I have pushed an update that allows clippy::assertions_on_constants crate-wide, due to the false positives.

(Allowing the individual asserts would be possible, but IMO the lint is pretty low-value so I don’t mind the blanket allow in this case.)

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2026
@Kobzol

Kobzol commented Jul 22, 2026

Copy link
Copy Markdown
Member

Yeah, agreed that the global allow is fine. I'll r+ once CI is green.

@Kobzol

Kobzol commented Jul 22, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

Thanks!

@rust-bors

rust-bors Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 480c37d has been approved by Kobzol

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 22, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 22, 2026
bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests

- [Zulip thread: Reducing conditional compilation in bootstrap?](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Reducing.20conditional.20compilation.20in.20bootstrap.3F/with/611882645)
---

Bootstrap contains a few code paths that need to be stubbed out when running bootstrap's unit tests, so that they don't download files or cause other problems. (That's unfortunate, but fixing it is outside the scope of this PR.)

Currently those code paths are skipped using `#[cfg(test)]` and `#[cfg(not(test))]`, which does achieve the desired effect, but has a few downsides. The stubbed-out code triggers dead-code and unused-import warnings, and makes IDE navigation harder when rust-analyzer treats `cfg(test)` as true when analyzing the code.

If we switch to using `cfg!(test)` and `cfg!(not(test))` to “dynamically” skip the relevant code paths, it still gets skipped, but the skipped code is still considered live for the purpose of dead-code warnings and IDE analysis.
rust-bors Bot pushed a commit that referenced this pull request Jul 22, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #159712 (Subtree sync for rustc_codegen_cranelift)
 - #155697 (Stabilize c-variadic function definitions)
 - #159285 (Simplify `apply_effects_in_range`)
 - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes)
 - #159659 (Move `Limit` out of `rustc_hir`)
 - #159451 (Remove config cloning in compiletest)
 - #159646 (Increase depth for float infer var fallback hack)
 - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 22, 2026
bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests

- [Zulip thread: Reducing conditional compilation in bootstrap?](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Reducing.20conditional.20compilation.20in.20bootstrap.3F/with/611882645)
---

Bootstrap contains a few code paths that need to be stubbed out when running bootstrap's unit tests, so that they don't download files or cause other problems. (That's unfortunate, but fixing it is outside the scope of this PR.)

Currently those code paths are skipped using `#[cfg(test)]` and `#[cfg(not(test))]`, which does achieve the desired effect, but has a few downsides. The stubbed-out code triggers dead-code and unused-import warnings, and makes IDE navigation harder when rust-analyzer treats `cfg(test)` as true when analyzing the code.

If we switch to using `cfg!(test)` and `cfg!(not(test))` to “dynamically” skip the relevant code paths, it still gets skipped, but the skipped code is still considered live for the purpose of dead-code warnings and IDE analysis.
rust-bors Bot pushed a commit that referenced this pull request Jul 22, 2026
…uwer

Rollup of 12 pull requests

Successful merges:

 - #159712 (Subtree sync for rustc_codegen_cranelift)
 - #155697 (Stabilize c-variadic function definitions)
 - #159285 (Simplify `apply_effects_in_range`)
 - #159596 (unify the AST repr of type const and const RHS)
 - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes)
 - #159659 (Move `Limit` out of `rustc_hir`)
 - #159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern)
 - #158479 (Reject static item as direct const generic arg)
 - #158738 (next_trait_solver: Recover from GCE const exprs)
 - #159451 (Remove config cloning in compiletest)
 - #159646 (Increase depth for float infer var fallback hack)
 - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)
rust-bors Bot pushed a commit that referenced this pull request Jul 22, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #159712 (Subtree sync for rustc_codegen_cranelift)
 - #155697 (Stabilize c-variadic function definitions)
 - #159285 (Simplify `apply_effects_in_range`)
 - #159596 (unify the AST repr of type const and const RHS)
 - #159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes)
 - #159659 (Move `Limit` out of `rustc_hir`)
 - #159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern)
 - #158738 (next_trait_solver: Recover from GCE const exprs)
 - #159451 (Remove config cloning in compiletest)
 - #159646 (Increase depth for float infer var fallback hack)
 - #159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)
@rust-bors
rust-bors Bot merged commit 6d43869 into rust-lang:main Jul 22, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 22, 2026
rust-timer added a commit that referenced this pull request Jul 22, 2026
Rollup merge of #159705 - Zalathar:cfg-test, r=Kobzol

bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests

- [Zulip thread: Reducing conditional compilation in bootstrap?](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Reducing.20conditional.20compilation.20in.20bootstrap.3F/with/611882645)
---

Bootstrap contains a few code paths that need to be stubbed out when running bootstrap's unit tests, so that they don't download files or cause other problems. (That's unfortunate, but fixing it is outside the scope of this PR.)

Currently those code paths are skipped using `#[cfg(test)]` and `#[cfg(not(test))]`, which does achieve the desired effect, but has a few downsides. The stubbed-out code triggers dead-code and unused-import warnings, and makes IDE navigation harder when rust-analyzer treats `cfg(test)` as true when analyzing the code.

If we switch to using `cfg!(test)` and `cfg!(not(test))` to “dynamically” skip the relevant code paths, it still gets skipped, but the skipped code is still considered live for the purpose of dead-code warnings and IDE analysis.
@Zalathar
Zalathar deleted the cfg-test branch July 23, 2026 00:16
Kobzol pushed a commit to Kobzol/rustc_codegen_cranelift that referenced this pull request Jul 23, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#159712 (Subtree sync for rustc_codegen_cranelift)
 - rust-lang/rust#155697 (Stabilize c-variadic function definitions)
 - rust-lang/rust#159285 (Simplify `apply_effects_in_range`)
 - rust-lang/rust#159596 (unify the AST repr of type const and const RHS)
 - rust-lang/rust#159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes)
 - rust-lang/rust#159659 (Move `Limit` out of `rustc_hir`)
 - rust-lang/rust#159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern)
 - rust-lang/rust#158738 (next_trait_solver: Recover from GCE const exprs)
 - rust-lang/rust#159451 (Remove config cloning in compiletest)
 - rust-lang/rust#159646 (Increase depth for float infer var fallback hack)
 - rust-lang/rust#159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)
moabo3li pushed a commit to moabo3li/miri that referenced this pull request Jul 23, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#159712 (Subtree sync for rustc_codegen_cranelift)
 - rust-lang/rust#155697 (Stabilize c-variadic function definitions)
 - rust-lang/rust#159285 (Simplify `apply_effects_in_range`)
 - rust-lang/rust#159596 (unify the AST repr of type const and const RHS)
 - rust-lang/rust#159607 (test: update riscv32e-registers.rs for LLVM 24 MC diagnostic changes)
 - rust-lang/rust#159659 (Move `Limit` out of `rustc_hir`)
 - rust-lang/rust#159707 (fix error when a dangling ref in a ManuallyDrop is used in a pattern)
 - rust-lang/rust#158738 (next_trait_solver: Recover from GCE const exprs)
 - rust-lang/rust#159451 (Remove config cloning in compiletest)
 - rust-lang/rust#159646 (Increase depth for float infer var fallback hack)
 - rust-lang/rust#159705 (bootstrap: Prefer `cfg!(not(test))` when skipping code paths during unit tests)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants